Skip to content

Allow Upper Case property names#1037

Merged
soyuka merged 1 commit into
api-platform:masterfrom
insekticid:patch-1
Apr 8, 2017
Merged

Allow Upper Case property names#1037
soyuka merged 1 commit into
api-platform:masterfrom
insekticid:patch-1

Conversation

@insekticid

Copy link
Copy Markdown
Contributor

ReflectionExtractor::getProperties() returns $id instead of $Id. It is bad naming convention, but is possible

use ApiPlatform\Core\Annotation\ApiProperty;
use Doctrine\ORM\Mapping as ORM;

class Entity {
    /**
     * @var string The dummy Guid
     *
     * @ORM\Column(nullable=true)
     * @ApiProperty()
     */
    protected $Id;

    public function getId()
    {
        return $this->Id;
    }
}
Q A
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets #1026
License MIT

@insekticid

Copy link
Copy Markdown
Contributor Author

I forgot to modify Behat scenarios

@dunglas

dunglas commented Apr 7, 2017

Copy link
Copy Markdown
Member

I strongly encourage you to test it with a new entity instead of modifying an existing one. It will be less painful than editing Behat tests.

ReflectionExtractor::getProperties() returns $id instead of $Id. It is bad naming convention, but is possible

```
use ApiPlatform\Core\Annotation\ApiProperty;
use Doctrine\ORM\Mapping as ORM;

class Entity {
    /**
     * @var string The dummy Guid
     *
     * @Orm\Column(nullable=true)
     * @ApiProperty()
     */
    protected $Id;

    public function getId()
    {
        return $this->Id;
    }
}
```
/**
* Custom identifier dummy.
*
* @author Exploit.cz <insekticid@exploit.cz>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol no I wondered the same

IMO we should keep as @author the one who created the class not uppon each edit :). Should be fixed in other classes too. Still, what's the point of adding @author when we can git blame?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@insekticid I have no problems with the @author tag but you made no changes on this class! 😄
Add it to the UpperCaseIdentifierDummy class instead, it's more relevant! 😉

@Simperfit

Copy link
Copy Markdown
Contributor

Shouldn't we add a behat test case to check ? @dunglas

@soyuka

soyuka commented Apr 7, 2017

Copy link
Copy Markdown
Member

@Simperfit no need to add a behat test here IMO, our behat suite is heavy enough (and painful to maintain too). The unit test covers the use case just fine!

@insekticid

insekticid commented Apr 7, 2017 via email

Copy link
Copy Markdown
Contributor Author

@dunglas

dunglas commented Apr 8, 2017

Copy link
Copy Markdown
Member

Don't worry I can do the change during the merge.
Thanks you very much for fixing this.

@soyuka
soyuka merged commit 350d050 into api-platform:master Apr 8, 2017
@soyuka

soyuka commented Apr 8, 2017

Copy link
Copy Markdown
Member

Thanks @insekticid !

fabpot added a commit to symfony/symfony that referenced this pull request Apr 29, 2017
…ticid)

This PR was merged into the 2.8 branch.

Discussion
----------

Allow Upper Case property names in ObjectNormalizer

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22547
| License       | MIT

Same problem that has been fixed here #22265
and here api-platform/core#1037

ObjectNormalizer returns $id instead of $Id. It is bad naming convention, but is possible

```php
class Entity {
    protected $Id;

    public function getId()
    {
        return $this->Id;
    }
}
```

Commits
-------

b2b4faa Allow Upper Case property names in ObjectNormalizer
symfony-splitter pushed a commit to symfony/serializer that referenced this pull request Apr 29, 2017
| Q                | A
| ---------------- | -----
| Bug report?      | yes
| Feature request? | no
| BC Break report? | yes
| RFC?             | no
| Symfony version  | 2.8.19

Same problem that has been fixed here symfony/symfony#22265
and here api-platform/core#1037

ObjectNormalizer returns $id instead of $Id. It is bad naming convention, but is possible

```php
class Entity {
    protected $Id;

    public function getId()
    {
        return $this->Id;
    }
}
```
symfony-splitter pushed a commit to symfony/serializer that referenced this pull request Apr 29, 2017
…ticid)

This PR was merged into the 2.8 branch.

Discussion
----------

Allow Upper Case property names in ObjectNormalizer

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22547
| License       | MIT

Same problem that has been fixed here symfony/symfony#22265
and here api-platform/core#1037

ObjectNormalizer returns $id instead of $Id. It is bad naming convention, but is possible

```php
class Entity {
    protected $Id;

    public function getId()
    {
        return $this->Id;
    }
}
```

Commits
-------

b2b4faa Allow Upper Case property names in ObjectNormalizer
hoangnd25 pushed a commit to hoangnd25/core that referenced this pull request Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants